-
Notifications
You must be signed in to change notification settings - Fork 2.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Upgrade ui #1066
Upgrade ui #1066
Conversation
@jojohappy nice work! Add an entry to chanelog please :) Do you have a docker image in dockerhub with this commit so that I can play around with it? |
Nice! |
Nice but it's kind of hard to review so many different (generated) files. Could you tell us how did you do this? Did you copy/paste over the relevant files? 😄 |
type="button" | ||
name="dec_end" | ||
title="Rewind the end time."> | ||
<i class="glyphicon glyphicon-backward"></i> | ||
</button> | ||
</button><!-- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we need this comment, for example? that's why I'm wondering 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just following the upstream (: I don't know why it was not deleted by Prometheus too. ):
@@ -15,15 +13,21 @@ | |||
|
|||
<script src="{{ pathPrefix }}/static/vendor/mustache/mustache.min.js?v={{ buildVersion }}"></script> | |||
<script src="{{ pathPrefix }}/static/vendor/js/jquery.selection.js?v={{ buildVersion }}"></script> | |||
<script src="{{ pathPrefix }}/static/vendor/js/jquery.hotkeys.js?v={{ buildVersion }}"></script> | |||
<!-- <script src="{{ pathPrefix }}/static/vendor/js/jquery.hotkeys.js?v={{ buildVersion }}"></script> --> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why is this commented out too, for example?
@GiedriusS Maybe we could play around this PATCH by the image Because we are following the Prometheus ui, as you said, I copied and pasted the relevant files and upgraded the frontend vendors for bootstrap 4.0. Exclude the The biggest changes for this PR is bootstrap, so a lot of style of dom elements would be updated, such as navbar and labels. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this @jojohappy Looking good so far, will build docker image to test this out for all.
.gitignore
Outdated
@@ -1,6 +1,7 @@ | |||
/prometheus | |||
/thanos | |||
vendor/ | |||
!pkg/ui/static/vendor |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's change the upper vendor to /vendor/
then instead of this ?
Are we sure the security patch for query history issue is there as well? |
I'd love to continue with this PR since it would solve an important pain point. @jojohappy, could you please point me from which exact commit in https://github.com/prometheus/prometheus you've pulled this from? I'd love to double check. |
I've tested Thanos Rule and I get |
Thanks @GiedriusS ! I followed the ui of Prometheus v2.9.1: commit. |
Signed-off-by: jojohappy <sarahdj0917@gmail.com>
Signed-off-by: jojohappy <sarahdj0917@gmail.com>
Signed-off-by: jojohappy <sarahdj0917@gmail.com>
Signed-off-by: jojohappy <sarahdj0917@gmail.com>
Signed-off-by: jojohappy <sarahdj0917@gmail.com>
Signed-off-by: jojohappy <sarahdj0917@gmail.com>
@povilasv @GiedriusS I have updated the image for this PR: |
Awesome work 🎉 ! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Double checked with that commit and everything seems clean AFAICT, and it works much better with the updated versions! Thank you a lot ❤️
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, I resolved conflicts and will merge on green & deploy it to our clusters (:
Thanks for massive work on this @jojohappy !! Really great help ❤️ |
❤️ @jojohappy , can't wait to run it in production :) |
Changes
fixes: #1042
Upgrade the Thanos ui to the Prometheus 2.9.1, new style of ui theme would be used, although I don't like it.
Verification
To visit the Thanos Query and Rule page.
@adrien-f Please help me to verify the
store
page, you are the original committer, thanks!@bwplotka @povilasv Please help me to verify all of the pages, especially the
rule
page, I have not a rule component, thanks!